Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate escape codes in aws_ssm output from newer versions of Bash #1839

Merged

Conversation

dennisjlee
Copy link
Contributor

SUMMARY

aws_ssm - prevent escape codes from interfering with output

Fixes #1756

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

aws_ssm

ADDITIONAL INFORMATION

This disables the Readline feature enable-bracketed-paste which is enabled by default on Bash 5.1 and above. This was causing escape sequences like \x1b[?2004h\x1b[?2004l to get into the output from some operating systems (e.g. Amazon Linux).

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/c5746171a1dc4f8ba59a219caeae2dd1

ansible-galaxy-importer FAILURE in 4m 46s
✔️ build-ansible-collection SUCCESS in 12m 36s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 41s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 37s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 07s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 11m 26s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 11m 06s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 03s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 22s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 04s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 6m 10s
✔️ ansible-test-changelog SUCCESS in 4m 36s
✔️ ansible-test-splitter SUCCESS in 4m 45s
✔️ integration-community.aws-1 SUCCESS in 8m 12s
✔️ integration-community.aws-2 SUCCESS in 9m 03s
✔️ integration-community.aws-3 SUCCESS in 10m 52s
Skipped 19 jobs

@dennisjlee
Copy link
Contributor Author

Could any of the maintainers help me review this please? cc @alinabuzachis @tremble

@alinabuzachis
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/a46899fa3ea8408e98551c68559eba3b

ansible-galaxy-importer FAILURE in 4m 06s
✔️ build-ansible-collection SUCCESS in 12m 55s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 26s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 20s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 12m 59s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 57s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 11m 00s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 25s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 47s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 16s
ansible-test-units-amazon-aws-python310 FAILURE in 6m 28s (non-voting)
✔️ ansible-test-changelog SUCCESS in 4m 48s
✔️ ansible-test-splitter SUCCESS in 4m 42s
✔️ integration-community.aws-1 SUCCESS in 8m 56s
✔️ integration-community.aws-2 SUCCESS in 9m 21s
✔️ integration-community.aws-3 SUCCESS in 23m 53s
Skipped 19 jobs

@richardsonky
Copy link

I have tested in our environment against several OS versions and it fixes the root problem. Please approve this and merge it.

@alinabuzachis
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/6799e5046c784865be991cd3de7a1041

ansible-galaxy-importer FAILURE in 6m 14s (non-voting)
✔️ build-ansible-collection SUCCESS in 12m 32s
✔️ ansible-test-splitter SUCCESS in 5m 08s
✔️ integration-community.aws-1 SUCCESS in 11m 35s
✔️ integration-community.aws-2 SUCCESS in 9m 15s
✔️ integration-community.aws-3 SUCCESS in 26m 52s
Skipped 19 jobs

@richardsonky
Copy link

what needs to be done to get this approved? PRs are taking a long time to get approved even though we've got several people saying it has fixed their problem.

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review connection connection plugin has_issue needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review labels Jul 14, 2023
@ansibullbot
Copy link

@dennisjlee this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed community_review small_patch Hopefully easy to review labels Jul 18, 2023
@dennisjlee
Copy link
Contributor Author

bot_status

@dennisjlee dennisjlee force-pushed the dj/disableBracketedPaste branch from 71940e9 to 3fc0fce Compare July 18, 2023 21:32
@dennisjlee
Copy link
Contributor Author

bot_status

@ansibullbot
Copy link

Components

changelogs/fragments/1839-disable-bracketed-paste.yml
support: community
maintainers:

plugins/connection/aws_ssm.py
support: community
maintainers:

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: blocked
shippable_status: None
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

@ansibullbot ansibullbot removed the merge_commit This PR contains at least one merge commit. Please resolve! label Jul 18, 2023
@richardsonky
Copy link

I'm not sure why, but the maintainers don't seem to be willing to merge these kinds of changes. I've tried in the past to fix problems but they never got approved. What i've started doing is to keep a local copy of the plugin in my repository. You can just create a folder in the root called 'connection_plugins' and then copy the file into this folder with the needed changes. Then in the playbook, set a variable called 'ansible_connection: filename' (don't include the extention). This allows ansible to use your local copy of the playbook and you can update it with any fixes that are not being included.

@simon97k
Copy link

Hi,
any info why this is on hold at the moment? This is still required to run Ansible with e.g. AWS Linux 2023 which is the default OS for EC2 instances in AWS.

@dennisjlee
Copy link
Contributor Author

Hi, any info why this is on hold at the moment? This is still required to run Ansible with e.g. AWS Linux 2023 which is the default OS for EC2 instances in AWS.

@simon97k I have no idea, sorry. I have not had any engagement from maintainers of this project after getting approvals earlier this year. I do not know what else is needed to get this merged. Any guidance from maintainers would be appreciated!

@jsubirat
Copy link

Hi @alinabuzachis , @hakbailey , any chance of getting this merged soon?

@ghost
Copy link

ghost commented Dec 27, 2023

Guess we wont' see progress on this in 2023? Shall we wait for Amazon Linux 2024 to get released?

@dennisjlee
Copy link
Contributor Author

recheck

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/d5d9a7b3b73e402a836a0598c36f6129

✔️ ansible-galaxy-importer SUCCESS in 4m 51s (non-voting)
✔️ build-ansible-collection SUCCESS in 14m 10s
✔️ ansible-test-splitter SUCCESS in 6m 52s
✔️ integration-community.aws-1 SUCCESS in 8m 29s
✔️ integration-community.aws-2 SUCCESS in 8m 07s
✔️ integration-community.aws-3 SUCCESS in 7m 13s
Skipped 19 jobs

@dennisjlee
Copy link
Contributor Author

@alinabuzachis and @hakbailey - the tests finally passed on this branch. Would either of you mind merging this please?

It would be great to get this fixed - it's been more than 7 months since I opened the PR.

@tremble tremble added backport-7 PR should be backported to the stable-7 branch backport-6 PR should be backported to the stable-6 branch and removed backport-6 PR should be backported to the stable-6 branch labels Jan 3, 2024
@tremble
Copy link
Contributor

tremble commented Jan 3, 2024

Based on #1793 this appears to be working for CentOS 9, Ubuntu and Amazon Linux 2. There exists a risk that this might break older OSes, however with CentOS 8 now EOL we don't have the ability to test this.

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Jan 3, 2024
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/4815e94bc6d548898c2516977cf4965d

✔️ ansible-galaxy-importer SUCCESS in 4m 25s (non-voting)
✔️ build-ansible-collection SUCCESS in 14m 47s
✔️ ansible-test-splitter SUCCESS in 5m 51s
✔️ integration-community.aws-1 SUCCESS in 9m 17s
✔️ integration-community.aws-2 SUCCESS in 9m 29s
✔️ integration-community.aws-3 SUCCESS in 8m 26s
Skipped 19 jobs

Copy link
Contributor

Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry.

@tremble tremble merged commit af18bc6 into ansible-collections:main Jan 3, 2024
72 of 99 checks passed
Copy link

patchback bot commented Jan 3, 2024

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/af18bc61c94003c146a31200c17b0b9cb0651823/pr-1839

Backported as #2029

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 3, 2024
…1839)

SUMMARY

aws_ssm - prevent escape codes from interfering with output

Fixes #1756

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

aws_ssm

ADDITIONAL INFORMATION

This disables the Readline feature enable-bracketed-paste which is enabled by default on Bash 5.1 and above. This was causing escape sequences like \x1b[?2004h\x1b[?2004l to get into the output from some operating systems (e.g. Amazon Linux).

(cherry picked from commit af18bc6)
Copy link

patchback bot commented Jan 3, 2024

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/af18bc61c94003c146a31200c17b0b9cb0651823/pr-1839

Backported as #2030

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 3, 2024
…1839)

SUMMARY

aws_ssm - prevent escape codes from interfering with output

Fixes #1756

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

aws_ssm

ADDITIONAL INFORMATION

This disables the Readline feature enable-bracketed-paste which is enabled by default on Bash 5.1 and above. This was causing escape sequences like \x1b[?2004h\x1b[?2004l to get into the output from some operating systems (e.g. Amazon Linux).

(cherry picked from commit af18bc6)
Copy link

github-actions bot commented Jan 3, 2024

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@richardsonky
Copy link

and there was much rejoicing!

@dennisjlee
Copy link
Contributor Author

Thank you @tremble!

softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jan 3, 2024
…1839) (#2030)

[PR #1839/af18bc61 backport][stable-7] Eliminate escape codes in aws_ssm output from newer versions of Bash

This is a backport of PR #1839 as merged into main (af18bc6).
SUMMARY
aws_ssm - prevent escape codes from interfering with output
Fixes #1756
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
aws_ssm
ADDITIONAL INFORMATION
This disables the Readline feature enable-bracketed-paste which is enabled by default on Bash 5.1 and above. This was causing escape sequences like \x1b[?2004h\x1b[?2004l to get into the output from some operating systems (e.g. Amazon Linux).

Reviewed-by: Mark Chappell
@tremble
Copy link
Contributor

tremble commented Jan 3, 2024

Sorry it took so long, the (full) integration tests for this plugin were broken when they took down the Fedora image it was using. Having previously spent way too long trying to get the test suite working I didn't have the time/energy to try and fix them at the time (not my plugin, I'm just a community maintainer who generally cares about things like the integration tests).

In #1793 I'm bumping them over to CentOS 9 because the cycle's longer and causes less pain. There exists a small risk that this PR will break compatibility with pre-bash-5.1 OSes, but I think the benefits outweigh the risks at this point.

community.aws 7.2 is due to be released "about now", so hopefully this fix will be available on galaxy in the not-too-distant future...

@brakf
Copy link

brakf commented Jan 3, 2024

thank you for the work you do and the effort to get this fixed

@docsaistvan
Copy link

@dennisjlee You are a LEGEND!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-6 PR should be backported to the stable-6 branch backport-7 PR should be backported to the stable-7 branch bug This issue/PR relates to a bug community_review connection connection plugin has_issue mergeit Merge the PR (SoftwareFactory) needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unexpected output from Python interpreter discovery with aws_ssm connection plugin